refactor(odoo): Core-first correction — structural facts home in the typed Core, harvest subordinate#530
Conversation
|
Warning Review limit reached
More reviews will be available in 55 minutes and 33 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c9cb383fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for kw in call.keywords: | ||
| if kw.arg == "selection": | ||
| sel = kw.value | ||
| break |
There was a problem hiding this comment.
Include selection_add keys in selection_value
When an addon extends an existing Odoo selection field with _inherit and fields.Selection(selection_add=[('x', 'X')]), these lines never inspect the statically known selection_add keyword, so the enrichment emits only the base field's values. That makes the generated selection_value set incomplete, and a downstream ASSERT $value IN [...] built from it would reject records using legal extension-added states.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 21d8ef0. Real finding — addons extend a Selection field's domain via _inherit + selection_add=[...] without redeclaring the base list. Two changes:
_extract_selection_valuesnow reads both the base list (positional /selection=) andselection_add=, unioned base-first with order-preserving dedup._scan_filebinding merges keys per(model, field)instead of last-write-wins — base andselection_addlive in different classes, so the value set accumulates across scans.
Cross-class regression test added: base state in the _name class + selection_add=[('reviewed', ...)] in an _inherit class → accumulated [draft, posted, reviewed]. 57/57 tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c9cb383fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for kw in call.keywords: | ||
| if kw.arg == "selection": | ||
| sel = kw.value | ||
| break |
There was a problem hiding this comment.
Include selection_add keys in selection_value
When an addon extends an existing Odoo selection field with _inherit and fields.Selection(selection_add=[('x', 'X')]), these lines never inspect the statically known selection_add keyword, so the enrichment emits only the base field's values. That makes the generated selection_value set incomplete, and a downstream ASSERT $value IN [...] built from it would reject records using legal extension-added states.
Useful? React with 👍 / 👎.
Adds a fifth enrichment pass to spo_enrich.py: `fields.Selection` declarations with a statically-resolvable list of 2-tuples emit one `(odoo:<model>.<field>, selection_value, "<key>")` triple per enum key. # Shape `_extract_selection_values(call)` pulls the first element of each 2-tuple from the Selection list — positional arg 0 OR `selection=` kwarg — preserving source order, de-duplicating (first occurrence wins). Skipped (values not statically knowable): - `selection='_compute_states'` (compute-method ref, a str) - `selection=STATE_CONSTANT` (bare Name) - `related=...` / any non-list/tuple selection arg - individual entries that aren't 2-tuples Truth `(0.95, 0.90)` — read straight from the field decorator, authoritative. Scoped to corpus-declared ObjectTypes (the additive boundary, same as P1); Selection fields bind to the same `model_names` as relational fields (`_name`, else `_inherit[0]`, per #525's decision). # Consumer use Lets odoo-rs lower a Selection field to `DEFINE FIELD state … ASSERT $value IN ['draft','posted','cancel']` — the UPSTREAM_WISHLIST P3 ask. The five-pass enrichment is now: P1 (target/inverse_name) + P0 (deep reads_field) + P1b (inherits_from) + P2 (validation_kind) + P3 (selection_value). # Corpus regen pending The shipped corpus does not yet carry selection_value triples — regenerating requires running the script against a live Odoo source tree (`/home/user/odoo/addons`), not present on this host. The Rust loader's predicate-histogram match arm gained `selection_value` so a future regenerated corpus drops into the harness without code changes. `parses_all_triples` count assertion stays at 24 579; re-locks the moment a session with the source re-runs enrichment. # Files spo_enrich.py: +`_extract_selection_values` helper, +`SELECTION_VALUE_TRUTH`, +`selections` param threaded through `_scan_file` / `build_all_facts` / `enrich`, +Selection branch in the field loop, +P3 emission loop, +CLI status field. test_spo_enrich.py: +12 tests (6 extraction edge cases + 3 scan-binding + 3 emission). odoo_ontology.rs: +doc table row, +histogram match arm for `selection_value`. EPIPHANIES.md: prepended E-ODOO-SPO-SELECTION-VALUE. # Tests python3 -m unittest tests.test_spo_enrich : 53/53 OK (was 41) cargo test -p lance-graph --lib odoo_ontology : 13/13 OK
0c9cb38 to
6ded9e0
Compare
…d Core, harvest is subordinate
Repurposes the P3 selection_value work after the operator caught a real
architectural inversion: the wishlist's structural asks (target /
inverse_name / inherits_from / selection_value) are CORE facts, not
behavioural harvest, and belong in the typed `OdooEntity` Core
(`lance-graph-ontology::odoo_blueprint`) — not bolted onto the flat SPO
ndjson by `spo_enrich.py`.
Source-verified: `OdooField.target` ALREADY exists in the Core, so the
`target` harvest pass was re-deriving a fact the Core already held;
`inherits_from` + `selection_value` were genuine Core gaps filled on the
harvest side — the "Core gap → extend the Core deliberately, never hack
the adapter" anti-pattern.
# Why a side-table, not a struct field
`OdooField` has 3 554 literal sites, `OdooEntity` 404, no constructor.
Adding a field to the mega-structs breaks every literal. The
doctrine-correct "extend the Core deliberately" for a literal sea that
size is a TYPED SIDE-TABLE — still Core, still authoritative, beside the
mega-structs instead of inside them.
# What lands
New `odoo_blueprint/structural.rs`:
- `OdooInherits { model, bases }` — the _inherit/_inherits mixin chain
(OdooEntityKind records the ORM base class, not the mixin list)
- `OdooFieldSelection { model, field, values }` — Selection value domain
(OdooFieldKind::Selection flags it; OdooField never stored the values)
- Curated `account.move` consts, GROUNDED: INHERITS matches the #527
corpus (mail_activity_mixin + sequence_mixin); FIELD_SELECTIONS uses
canonical standard Odoo state/move_type value sets.
- `project_inherits_from` / `project_selection_value` — Core → SPO
projection (direction of truth: Core out to SPO, never the reverse).
- 5 tests incl. a consistency check against the #527 corpus.
# Reframing (anti-self-fulfilling-drift)
So a future session does NOT read the prior work as "add another
spo_enrich predicate":
- `spo_enrich.py` module-doc gains an ARCHITECTURE NOTE: the structural
predicates' authoritative home is the typed Core; this file is the
Extracted-leg BREADTH feeder for the ~322 ObjectTypes the curated
Core hasn't reached; Curated Core wins on convergence. Behavioural
predicates (deep reads_field, emitted_by, validation_kind) ARE
genuine harvest and correctly stay.
- `EPIPHANIES.md` prepends E-ODOO-CORE-FIRST-STRUCTURAL, which
explicitly BOUNDS THE FRAMING of the four prior E-ODOO-SPO-* entries
so the predicate-bolt-on cadence isn't read as the intended
architecture. Names `virtually_overrides` as a ClassView/Core
capability — NOT harvest predicate #6.
# Not changed
The selection_value AST reader in spo_enrich.py stays (it's the Extracted
leg's mechanism for breadth) — now correctly subordinate to the Core.
Behavioural harvest is untouched.
# Tests
cargo test -p lance-graph-ontology --lib structural : 5/5
python3 -m unittest tests.test_spo_enrich : 53/53
cargo test -p lance-graph --lib odoo_ontology : 13/13 (unchanged)
Odoo addons extend an existing Selection field's domain via
`_inherit` + `fields.Selection(selection_add=[('reviewed','Reviewed')])`
WITHOUT redeclaring the base list. `_extract_selection_values` only read
the base `selection=` arg, so the emitted `selection_value` set was
incomplete and a downstream `ASSERT $value IN [...]` would reject records
in legal extension-added states.
Two fixes:
1. `_extract_selection_values` now reads BOTH the base list (positional
arg 0 or `selection=`) AND `selection_add=`, unioning base-first with
order-preserving dedup (factored into `_selection_tuple_keys`).
2. `_scan_file` binding MERGES selection keys per (model, field) instead
of last-write-wins — the base declaration and the `selection_add`
extension live in DIFFERENT classes, so the field's value set must
accumulate across scans, not overwrite.
Tests (53 -> 57):
- selection_add kwarg alone -> added keys
- base + selection_add -> union, base first
- selection_add dedups against base
- cross-class merge: base in _name class + selection_add in _inherit
class -> accumulated [draft, posted, reviewed]
python3 -m unittest tests.test_spo_enrich : 57/57 OK
fix(odoo-spo): read selection_add + merge across classes (codex #530 follow-up)
Summary
Repurposed. This started as "add
selection_valueasspo_enrich.pypredicate #5 (wishlist P3)." The operator caught a real architectural inversion before merge, so it is now the Core-first correction: the wishlist's structural asks belong in the typedOdooEntityCore (lance-graph-ontology::odoo_blueprint), not bolted onto the flat SPO ndjson by the AST harvest.The inversion (source-verified)
target/inverse_name/inherits_from/selection_valueare Core facts — relations, composition, value-domain — per the Core-first transcode doctrine. Checking the actual typed Core:OdooEntityCoretarget/inverse_nameOdooField.targetalready existsinherits_fromOdooEntityKind= ORM base class only, no mixin listselection_valueOdooFieldKind::Selectionflags it; no values slotFilling a Core gap on the harvest side is the doctrine's named anti-pattern: "a Core gap → extend the Core deliberately, never hack the adapter."
Why a typed side-table (not a struct field)
OdooFieldhas 3 554 literal sites,OdooEntity404, no constructor — adding a field to the mega-structs breaks every literal. The doctrine-correct "extend the Core deliberately" for a literal sea that size is a typed side-table: still Core, still authoritative, beside the mega-structs.What lands
New
crates/lance-graph-ontology/src/odoo_blueprint/structural.rs:OdooInherits { model, bases }— the_inherit/_inheritsmixin chainOdooFieldSelection { model, field, values }— the Selection value domainaccount.moveconsts, grounded:INHERITSmatches the feat(odoo-spo): regenerate corpus with inherits_from + validation_kind (#526 follow-up) #527 corpus (mail_activity_mixin+sequence_mixin);FIELD_SELECTIONSuses canonical standard Odoostate/move_typesetsproject_inherits_from/project_selection_value— Core → SPO projection (direction of truth: Core out to SPO, never reverse)Reframing — so a future session does not read the prior work as "add another
spo_enrichpredicate":spo_enrich.pymodule-doc gains anARCHITECTURE NOTE: the structural predicates' authoritative home is the typed Core; this file is the Extracted-leg breadth feeder for the ~322 ObjectTypes the curated Core hasn't reached; the Curated Core wins on convergence in theSpoStore. Behavioural predicates (deepreads_field,emitted_by,validation_kind) ARE genuine harvest and correctly stay.EPIPHANIES.mdprependsE-ODOO-CORE-FIRST-STRUCTURAL, which explicitly bounds the framing of the four priorE-ODOO-SPO-*entries (append-only; they're retained, their predicate-bolt-on cadence is no longer the headline). Namesvirtually_overridesas a ClassView/Core capability — not harvest predicate feat(graph): add SPO triple store with bitmap ANN, TruthGate, semirin… #6.Two-leg convergence (the standing shape)
Per
odoo-extraction-strategies-v1.md: the Curated leg (typed Core: this module + the 66 L-doc entities) is authoritative; the Extracted leg (spo_enrich.py) is breadth for uncurated models, subordinate. Structural facts live in the Core and project out to SPO; the harvest fills in where the Core is silent. This is the intended architecture — the drift was presenting the harvest as the home, not the legs existing.What is unchanged
The
selection_valueAST reader inspo_enrich.pystays — it's the Extracted leg's mechanism for breadth — now correctly subordinate. The mergedinherits_from/validation_kind/target/deep-reads_fieldpasses (#523/#526/#527) are not reverted; their framing is corrected forward via the EPIPHANIES entry, and their structural outputs are now superseded-as-source by the typed Core where it covers a model.Tests
cargo test -p lance-graph-ontology --lib structural— 5/5python3 -m unittest tests.test_spo_enrich— 53/53cargo test -p lance-graph --lib odoo_ontology— 13/13 (unchanged)Follow-up (not in this PR)
structural.rsregistries beyondaccount.moveas L-doc curation reaches more models.virtually_overrides→ the typed Core's class-resolution surface (ClassView), not a harvest predicate.